-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add v4 chunk format to chunks-inspect tool #13057
base: main
Are you sure you want to change the base?
Conversation
Did the same in #11506. |
@agebhar1's PR is merged so I'm going to close this |
…'clean room' implementation able to decode chunks without importing any Loki code, this update undid that implementation and we would prefer to keep it as a "clean room" type application Signed-off-by: Edward Welch <edward.welch@grafana.com>
Signed-off-by: Edward Welch <edward.welch@grafana.com>
Signed-off-by: Edward Welch <edward.welch@grafana.com>
Signed-off-by: Edward Welch <edward.welch@grafana.com>
ff8665b
to
b8c7f51
Compare
re-opening this PR after reverting the other implementation, it's preferable that this tool does not import any Loki code to decompress chunks, rather it's able to do so by implementing the chunk format directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm
//expCRC := binary.BigEndian.Uint32(data[structuredMetadataOffset+structuredMetadataLength:]) | ||
//computedMetaChecksum := crc32.Checksum(lb, castagnoliTable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented code, looks like a left over
|
||
structuredMetadata := data[structuredMetadataOffset : structuredMetadataOffset+structuredMetadataLength] | ||
|
||
// Structured Metadata is "normalized" or "compressed" by storing an index to a string with each log line, and then all the strings in the chunk metadata section |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: By reading the code not sure I understand the "normalized" part of this comment, but maybe I'm just missing context
What this PR does / why we need it:
The chunks-inspect tool was never updated to parse the newer v4 chunk with structured metadata, this PR fixes that
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)feat
PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PRdeprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR